Skip to content

Introduce the RetryPolicy abstraction#2

Draft
stIncMale wants to merge 5 commits into
refactorRetriesInMixedBulkWritefrom
introduceRetryPolicy
Draft

Introduce the RetryPolicy abstraction#2
stIncMale wants to merge 5 commits into
refactorRetriesInMixedBulkWritefrom
introduceRetryPolicy

Conversation

@stIncMale

@stIncMale stIncMale commented Jun 4, 2026

Copy link
Copy Markdown
Owner

This PR existed for demonstration purposes. The actual PR is mongodb#2004.

Notable changes:

  • LoopState -> LoopControl.
  • RetryState -> RetryControl.
  • onAttemptFailureOperator and retryPredicate in RetryState->RetryControl were replaced by RetryPolicy.onAttemptFailure.
    • An accidental consequence is that the logic that was in retryPredicate is now called on every failed attempt, unlike previously, where only onAttemptFailureOperator was called on every failed attempt. As a result, we no longer need to call CommandOperationHelper.addRetryableLabelOrGetWriteAttemptFailureNotToBeRetried->SpecRetryPolicy.decideRetryableAndAddRetryableWriteErrorLabelIfNeeded explicitly in MixedBulkWriteOperation/ClientBulkWriteOperation, or use AsyncOperationHelper.addingRetryableLabelCallback/CommandOperationHelper.addRetryableWriteErrorLabel in AsyncOperationHelper.executeRetryableWriteAsync/SyncOperationHelper.executeRetryableWrite.
  • RetryingAsyncCallbackSupplier, RetryingSyncSupplier no longer pass Errors to advanceOrThrow.
  • RetryingSyncSupplier passes Exceptions to advanceOrThrow as is, without wrapping them in RuntimeException. Wrapping is done by RetryState->RetryControl, and only if that checked Exception has to be thrown.
  • AttachmentKey and the related methods and implementations were deleted. Instead, SpecRetryPolicy now exposes methods with more specific semantics and more specific assertions guarding their usage.
  • AttachmentKeys
    • commandDescriptionSupplier -> SpecRetryPolicy.onCommand instead of
    • retryableWriteCommandFlag&maxWireVersion -> SpecRetryPolicy.onWriteRetryRequirements
    • command -> was not needed at all, and was replaced with a local MutableValue<BsonDocument> variable.
  • Since SpecRetryPolicy implements both read and write retry logic, the methods AsyncOperationHelper.decorateReadWithRetriesAsync/decorateWriteWithRetriesAsync and SyncOperationHelper.decorateReadWithRetries/decorateWriteWithRetries were replaced with a single method AsyncOperationHelper.decorateWithRetriesAsync and SyncOperationHelper.decorateWithRetries respectively.
  • RetryState->RetryControl.doWhileDisabled/doWhileDisabledAsync was introduced to replace the broken ClientBulkWriteOperation.doWithRetriesDisabled/doWithRetriesDisabledAsync.
  • AsyncOperationhelper.executeRetryableWriteAsync was rewritten with the beginAsync API. Now it is an even more straightforward translation of SyncOperationHelper.executeRetryableWrite.
    • More importantly, this allowed us to delete RetryState->RetryControl.breakAndCompleteIfRetryAnd, and use only the breakAndThrowIfRetryAnd method in both sync and async code.
  • RetryState->RetryControl.breakAndThrowIfRetryAnd is now used only to break from a retry attempt based on the information that could not have been available before the attempt start. The only such information is the information about the server selected, which is checked by the OperationHelper.canRetryWrite->isServerWriteRetryRequirementsMet method.
    • Read commands have not been checking that information, so RetryState->RetryControl.breakAndThrowIfRetryAnd is now used only by writes.
  • BulkWriteBatch
    • getRetryWrites -> isWriteRetryRequirementsMet
    • isRetryable -> isCommandWriteRetryRequirementsMet
  • OperationHelper
    • isRetryableWrite -> isNonCommandWriteRetryRequirementsMet
    • canRetryWrite -> isServerWriteRetryRequirementsMet
  • CommandOperationHelper
    • onRetryableReadAttemptFailure -> became part of SpecRetryPolicy.onAttemptFailure
    • chooseRetryableReadException -> SpecRetryPolicy.decideReadProspectiveFailedResult
    • onRetryableWriteAttemptFailure -> became part of SpecRetryPolicy.onAttemptFailure
    • chooseRetryableWriteException -> SpecRetryPolicy.decideWriteProspectiveFailedResult
    • initialRetryState -> createSpecRetryControl
    • loggingShouldAttemptToRetryRead -> became part of SpecRetryPolicy.onAttemptFailure, isRetryableReadError.
    • loggingShouldAttemptToRetryWriteAndAddRetryableLabel -> became part of SpecRetryPolicy.onAttemptFailure
    • addRetryableLabelOrGetWriteAttemptFailureNotToBeRetried -> SpecRetryPolicy.decideRetryableAndAddRetryableWriteErrorLabelIfNeeded
    • isRetryableWriteCommand -> isWriteRetryRequirementsMet
    • decideRetryableAndAddRetryableWriteErrorLabel -> became part of SpecRetryPolicy.decideRetryableAndAddRetryableWriteErrorLabelIfNeeded
    • addRetryableWriteErrorLabel -> addRetryableWriteErrorLabelIfNeeded
    • logRetryCommand -> SpecRetryPolicy.onAttemptStart
    • logUnableToRetryCommand -> SpecRetryPolicy.logUnableToRetryError (also, SpecRetryPolicy.logUnableToRetryMaxAttemptsReached was added)

JAVA-6229

@stIncMale stIncMale self-assigned this Jun 4, 2026
@stIncMale stIncMale force-pushed the introduceRetryPolicy branch 5 times, most recently from 1483625 to 4a1aa2b Compare June 16, 2026 01:15
@stIncMale stIncMale force-pushed the introduceRetryPolicy branch 2 times, most recently from 759cbbf to d62866c Compare June 18, 2026 23:27
@stIncMale stIncMale force-pushed the introduceRetryPolicy branch 6 times, most recently from f1cf803 to 8cacb8c Compare June 20, 2026 00:14
@stIncMale stIncMale force-pushed the introduceRetryPolicy branch from 8cacb8c to 48ee5ca Compare June 20, 2026 00:51
@stIncMale stIncMale force-pushed the introduceRetryPolicy branch from 48ee5ca to 69ad9b4 Compare June 22, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant